Delimit rangeproof cache key fields to prevent boundary collisions - #1601

Summary

ComputeEntryRangeProof builds the rangeproof cache key by concatenating four fields with no delimiters. Two of those fields are variable-length and sit at opposite ends of the stream, so two different (proof, value_commitment, asset_commitment, scriptPubKey) tuples can produce the same cache key. A positive cache hit returns true without verifying, so a node that has cached one valid rangeproof can accept a different, unverified proof. This PR makes the key encoding injective by committing to the field lengths, and adds a regression test.

The bug, in plain terms

The cache key is the hash of four pieces glued end-to-end with nothing between them. Picture the glued result as 8 letters, always in the same order:

A B C D E F G H

Rule of the format: the two middle pieces are a fixed 2 letters each; the first and last piece can be any length.

A normal transaction cuts them like this:

[A]     [BC]    [DE]    [FGH]
proof    vc      ac     script

The attacker re-cuts the same 8 letters like this:

[ABC]   [DE]    [FG]    [H]
proof    vc      ac     script

Both cuts glue back to the identical byte string ABCDEFGH. The key is the hash of the glued bytes, so both cuts hash to the same key. even though one is proof A / script FGH and the other is proof ABC / script H. Nothing was moved to the front; only the invisible boundary between "proof" and "script" slid to the right. The hash saw the bytes but never the boundaries.

This is not a SHA-256 collision. Two different logical inputs are made to produce the same byte string that gets hashed; identical input, identical output, as expected. The weakness is in how the pre-image is assembled, not in the hash.

Why it matters

A positive cache entry short-circuits verification: rangeProofCache.Get(entry, !store) returns true and VerifyRangeProof returns without ever checking the proof.

An accepted-but-unverified rangeproof means a hidden output amount is taken on trust, which breaks the confidential-value balance guarantee.

The fix

Prepend the four field lengths to the hashed key:

const uint64_t lengths[4] = {proof.size(), commitment.size(), asset_commitment.size(), scriptPubKey.size()};
hasher.Write(reinterpret_cast<const unsigned char*>(lengths), sizeof(lengths)) /* then the fields as before */;

Now the two cuts differ before the bytes even start:

normal:  hash( 1,2,2,3  ‖  ABCDEFGH )
attack:  hash( 3,2,2,1  ‖  ABCDEFGH )

Different lengths → different key → cache miss → the bogus proof is verified for real and rejected. The encoding is now injective: no other partition of the same stream yields the same key. The key is process-local (salted per start, never serialized or compared across nodes), so the length array's native byte order is fine.

Relationship to the earlier cache-key change

c26d719c29 (bind the rangeproof cache key to the asset commitment and scriptPubKey) is necessary but not sufficient: it added those fields to the key but still concatenated everything without delimiters, leaving this boundary ambiguity in place. This PR closes it.

Test

blind_tests/rangeproof_cache_key_field_boundary primes the cache with a genuine proof over a 69-byte OP_RETURN script (6a 43 ‖ C(33) ‖ X(33) ‖ 6a), then submits the attack tuple with the boundary shifted 68 bytes; the attack proof is invalid for its 1-byte script and must be rejected.

Verified by toggling only the fix: with the change the test passes; reverting just the sigcache.cpp hunk makes it fail (the bogus proof is wrongly accepted); restoring it passes again.

Notes

The fix was based on the Bug B reported on https://gist.github.com/1440000bytes/211ac92dd4433bb1a2e674bf0ff7db2e
by @1440000bytes. And also an analysis over logs of an Elements node.